-
Notifications
You must be signed in to change notification settings - Fork 6
Move off deprecated log events #90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
2d2d8d8
to
1974277
Compare
@anuraaga Could you please take a look? Should I just remove the beta chat completions instrumentation? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM --> The CI changes
@xrmx Personally I would remove the beta chat instrumentation indeed since I think it's superceded by the responses API |
instrumentation/elastic-opentelemetry-instrumentation-openai/pyproject.toml
Outdated
Show resolved
Hide resolved
But the current version ceiling looks fine too |
@xrmx Sorry I realized I needed to refresh my venv when testing. Actually I still had an issue when trying it out
AFAICT, this is because of open-telemetry/opentelemetry-python#4319 I came up with a workaround to force use the SDK I also had to work around severity_number crash which is probably an easier to fix bug in the logs exporter
Given the logs SDK doesn't seem to be working well currently, should we hold off on this PR? I recommend splitting off the |
@anuraaga I think the ceiling fix is required just for CI and wrapt would just ignore silently stuff that was not able to wrap. |
@anuraaga rebased and bumped baseline after fixing Logger.emit for API Logrecord upstream. |
...tic-opentelemetry-instrumentation-openai/src/opentelemetry/instrumentation/openai/helpers.py
Show resolved
Hide resolved
...tic-opentelemetry-instrumentation-openai/src/opentelemetry/instrumentation/openai/helpers.py
Show resolved
Hide resolved
any chance we can land this before elasticon NYC? (like in a week)? there's some significant tech debt until this is merged and released. |
Yeah, we should be able do that |
What does this pull request do?
This PR moves to OpenTelemetry 1.35.0 as baseline to move off deprecated log events and use log records directly. The telemetry output does not change.
Related issues
Closes #85